ci: add Windows workflow#1
Conversation
|
You are seeing this message because GitHub Code Scanning has recently been set up for this repository, or this pull request contains the workflow file for the Code Scanning tool. What Enabling Code Scanning Means:
For more information about GitHub Code Scanning, check out the documentation. |
bd4a355 to
0aeac2e
Compare
0aeac2e to
7a005b0
Compare
31ec05e to
8ebf211
Compare
8ebf211 to
e3da771
Compare
e38a085 to
19dd1dc
Compare
|
|
||
| machine: | ||
| name: machine (${{ matrix.provider }}) | ||
| needs: installer |
There was a problem hiding this comment.
Running e2e machine tests doesn't require the installer. The only prerequisites to run machine tests are podman and gvproxy binaries.
That also means that installer and machine tests can be split into 2 separate workflows.
There was a problem hiding this comment.
My suggestion was task 1 build the installer and upload, then task 2 machine test depend on it and download the artifact. So the machine tests should resuse all binaries from the installer so we actually test with the final product that users will use, otherwise we will never know if the installer ships broken things.
It also means we only need one task to build and uplaod and then two tasks who download and do not have to recompile again.
There was a problem hiding this comment.
@Luap99 the problem is that we need to update the source code to do that. e2e tests aren't designed to use the Podman binary from $PATH. Apart from that I agree that it would be better to run the machine e2e tests with a version of Podman installed with the instller.
But, if we do that (and I am +1 to update the code to do it), we need to split the build of the installer and the tests of the installer in two separate tasks. That's because we don't need to wait the completion of the installer tests to run the e2e tests.
There was a problem hiding this comment.
well that is what the PODMAN_BINARY env is far already, the test should not have to use $PATH, just set PODMAN_BINARY to wherever the installer installs it.
https://github.com/containers/podman/blob/c995882ecfe9ef45dcf63da17c8d84c3baa04858/pkg/machine/e2e/config_test.go#L114-L116
There was a problem hiding this comment.
If we set $PODMAN_BINARY we don't test that the installer updates $PATH. We don't verify the real user scenario. The installer can be broken, but the tests will pass just fine.
There was a problem hiding this comment.
Well the point of the machine tests is to test that the resulting binaries of the installer work, updating $PATH feels wrong because you can then never be sure what the test is using, could be a different podman from $PATH, maybe not a problem for windows but on linux that can be a problem.
I thought your installer tests cover the exact installer details? I.e. that should check for $PATH config on so on to ensure this part works while the machine tests should ensure they actually can be used to run podman machine
There was a problem hiding this comment.
Anyhow I guess we do not need to worry about the fine details to much, we can tweak behaviours later at any time.
I guess we can keep the current logic as is for the initial ci integration and the update the workflows later.
There was a problem hiding this comment.
I.e. that should check for $PATH config on so on to ensure this part works while the machine
FWIW my installer tests don't verify the $PATH update. We could do that, but it's not trivial because it requires starting a new session.
|
https://github.com/podman-io/podman-sandbox/actions/runs/26234555336/job/77203957305?pr=1 |
l0rd
left a comment
There was a problem hiding this comment.
A few minor comments but other than that LGTM 👍
d178581 to
aecc6b3
Compare
0f7a7e9 to
598dd94
Compare
Signed-off-by: Tim Zhou <tizhou@redhat.com>
598dd94 to
15e7b9c
Compare
Signed-off-by: Tim Zhou <tizhou@redhat.com>
Signed-off-by: Tim Zhou <tizhou@redhat.com>
ad185c5 to
ddd8d01
Compare
windows workflow